Skip to content

add bonus calendar proxy#381

Open
anjeshshrestha wants to merge 3 commits intodarkbot-reloaded:masterfrom
anjeshshrestha:bonuscalendar
Open

add bonus calendar proxy#381
anjeshshrestha wants to merge 3 commits intodarkbot-reloaded:masterfrom
anjeshshrestha:bonuscalendar

Conversation

@anjeshshrestha
Copy link
Copy Markdown
Contributor

@anjeshshrestha anjeshshrestha commented Nov 30, 2024

@Override
public void update() {
daysClaimed = readInt(0x30, 0x40);
rewardList.update(readAtom(0x30, 0x58));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the listing does not change, this will only be read once

Copy link
Copy Markdown
Contributor

@dm94 dm94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI] PR Review - Suggestions for improvement:\n\nBonusCalendarProxy.java\n- Line 19: claimable as �oolean is fine but consider using BonusCalendarAPI.ClaimState enum for future extensibility\n- Line 13: Memory offsets should include a comment noting which client version they were derived from\n- Line 25: Inner RewardLoot class visibility is acceptable if not externally accessed\n\nFacadeManager.java\n- Line 90: Minor formatting inconsistency - extra whitespace compared to surrounding code\n\nOverall the implementation looks solid and follows the existing proxy patterns in the codebase.

public void update() {
daysClaimed = readInt(0x30, 0x40);
rewardList.update(readAtom(0x30, 0x58));
claimable = readBoolean(0x30, 0x50, 0x20);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI] Consider using BonusCalendarAPI.ClaimState instead of primitive �oolean for better extensibility if more claim states are added in the future (e.g., ALREADY_CLAIMED, LOCKED, etc.). This would align with the API interface pattern.

@Getter
@ToString
private static class RewardLoot extends Updatable implements BonusCalendarAPI.RewardList {
private String lootId;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI] The inner RewardLoot class is marked private. If extensibility or testing is needed, consider making it package-private or protected. Currently private is fine for encapsulation if no external access is required.

@@ -88,6 +90,7 @@ public FacadeManager(PluginAPI pluginApi) {
this.worldBossOverview = registerProxy("worldBoss_overview", WorldBossOverviewProxy.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[AI] Minor: Inconsistent indentation/spacing around the new line. his.groupMediator has extra whitespace compared to other fields. Consider keeping consistent formatting with existing code style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants